Skip to content

feat(project): add online-eval to project add - #2048

Merged
nborges-aws merged 1 commit into
refactorfrom
feat/add-project-online-eval
Aug 20, 2026
Merged

feat(project): add online-eval to project add#2048
nborges-aws merged 1 commit into
refactorfrom
feat/add-project-online-eval

Conversation

@jariy17

@jariy17 jariy17 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Command structure

agentcore project add online-eval --help

Usage: agentcore project add online-eval [options]

adds an online evaluation config to the current project

Options:
  --name <name>                          the name of the online evaluation config
  --agent <agent>                        harness/runtime name whose traffic to sample (mutually exclusive with --log-group-name)
  --endpoint <endpoint>                  the agent endpoint qualifier to scope monitoring to (requires --agent)
  --log-group-name <log-group-name...>   CloudWatch log group name(s) for custom data sources (1-5; mutually exclusive with --agent)
  --service-name <service-name...>       service name(s) to filter traces for custom data sources (requires --log-group-name)
  --evaluator <evaluator...>             evaluator name(s), Builtin.* IDs, or ARNs to apply
  --sampling-rate <sampling-rate>        percentage of sessions to sample (0.01-100)
  --description <description>            a description of the config's monitoring purpose
  --enable-on-create <enable-on-create>  enable evaluation immediately after deploy (default true; pass false to add it paused)
  --tags <tags>                          tags to apply (JSON object of key/value strings)
  -h, --help                             display help for command

Related: aws/agentcore-l3-cdk-constructs#335 — OnlineEvaluationConfig agent resolves runtimes only (harness targets unsupported). The --agent help text here says "runtime" to match.

@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label Aug 19, 2026
@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.15%. Comparing base (33e2d2f) to head (a171b74).
⚠️ Report is 3 commits behind head on refactor.

Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #2048   +/-   ##
=========================================
  Coverage     97.14%   97.15%           
=========================================
  Files           382      383    +1     
  Lines         22884    22975   +91     
=========================================
+ Hits          22231    22321   +90     
- Misses          653      654    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Aug 19, 2026
Comment thread src/core/project/manager.tsx Outdated
});
break;
}
case "online-eval": {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a default for this makes sense to avoid having cases that all have no scaffolding and directly push. But at the same time, having the default loosens our validations since new resourceTypes will fall through to the default.

Meet in the middle: a resourceType enum, stack cases for new resourceTypes that do not need scaffolding

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look at your config bundle PR and follow that pattern.

@Hweinstock

Copy link
Copy Markdown
Contributor

Run bun run format:check
$ prettier --check .
Checking formatting...
[warn] src/handlers/project/add/online-eval/index.test.ts
[warn] src/handlers/project/add/online-eval/index.ts
[warn] Code style issues found in 2 files. Run Prettier with --write to fix.
error: script "format:check" exited with code 1

I think format is failing.

@Hweinstock Hweinstock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two nits on comments, otherwise lgtm (once format is fixed).

Comment thread src/core/project/manager.tsx Outdated
break;
}
case "online-eval": {
// Pure spec-level resource: no files to scaffold, just record the config.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i feel like this comment is expressed by the code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove it

// OnlineEvalConfigSchema owns every cross-field rule (agent XOR log groups,
// evaluators XOR insights, endpoint/serviceNames prerequisites). Validate once
// here so the user gets one actionable message rather than a raw stack later at
// deploy time — json.write does not re-validate on the way to disk.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this comment is fully accurate since we do actually validate before writing:

const newSpecParseResult = ProjectSpecSchema.safeParse(newSpec);
.

Its also referencing other parts of the codebase that may change, so its high risk to become stale.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just gonna tell claude to stop putting code comments unless there is a good reason too.

@jariy17
jariy17 force-pushed the feat/add-project-online-eval branch from 7edf52f to 2639903 Compare August 20, 2026 14:06
Comment thread src/core/project/manager.tsx Outdated
Comment on lines +158 to +160
case "online-eval":
newResources.push(resourceConfig);
break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets just stack this on config bundle

@jariy17
jariy17 force-pushed the feat/add-project-online-eval branch from 2639903 to a171b74 Compare August 20, 2026 16:24

@notgitika notgitika left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nborges-aws nborges-aws left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nborges-aws
nborges-aws merged commit 63ed69c into refactor Aug 20, 2026
11 checks passed
@nborges-aws
nborges-aws deleted the feat/add-project-online-eval branch August 20, 2026 16:42
notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 21, 2026
Upstream moved the per-resource `project add` tests out of the monolithic
project.test.ts into colocated add/<resource>/index.test.ts suites (harness
in aws#2034, online-eval in aws#2048). Move the memory tests to match, with the
same locally-duplicated run/inProject helpers those suites use.

project.test.ts is now identical to upstream/refactor again, so this PR no
longer touches it. Also drops the DeserializationError, FsReadWriteJson and
ReadWriteJson imports, left dead there once the harness tests that used them
moved to add/harness/index.test.ts.

No test content changed: 187 project tests still pass, now across 10 files
instead of 9.
notgitika added a commit to notgitika/agentcore-cli that referenced this pull request Aug 21, 2026
Upstream moved the per-resource `project add` tests out of the monolithic
project.test.ts into colocated add/<resource>/index.test.ts suites (harness
in aws#2034, online-eval in aws#2048). Move the memory tests to match, with the
same locally-duplicated run/inProject helpers those suites use.

project.test.ts is now identical to upstream/refactor again, so this PR no
longer touches it. Also drops the DeserializationError, FsReadWriteJson and
ReadWriteJson imports, left dead there once the harness tests that used them
moved to add/harness/index.test.ts.

No test content changed: 187 project tests still pass, now across 10 files
instead of 9.
jariy17 pushed a commit that referenced this pull request Aug 24, 2026
* feat(project): add `project add memory`

Registers a `memory` leaf under `project add`, following the same
SDK-union -> flat project-schema conversion pattern as `project add
harness`. A memory scaffolds no files, so the command only appends an
entry to `spec.memories` in agentcore.json; the L3 CDK turns that into
an `AWS::BedrockAgentCore::Memory` at deploy time.

Flags: --name, --event-expiry-duration, --strategies, --indexed-keys,
--stream-delivery-resources, --encryption-key-arn, --execution-role-arn,
--tags.

--strategies accepts two forms: a comma-separated list of strategy types
expanded with the CLI's default namespace templates, or a JSON
MemoryStrategyInput[] mirroring the CreateMemory API for strategies that
need explicit names, descriptions, or namespaces.

clientToken is excluded (it is CreateMemory idempotency and this command
makes no API call), and description is excluded until the L3 CDK schema
supports it.

* feat: add --description to 'project add memory'

Stores an optional memory description in agentcore.json, matching the
CreateMemory API's description field (max 4096 characters).

The generated CDK app pins @aws/agentcore-cdk 0.1.0-alpha.45, whose
MemorySchema is a non-strict z.object with no description field, so the key
is stripped at synth rather than rejected until
aws/agentcore-l3-cdk-constructs#325 ships and that pin is bumped. The flag
help text says so.

* feat: accept a CUSTOM memory strategy in 'project add memory'

The CDK's memory schema already models CUSTOM (@aws/agentcore-cdk
0.1.0-alpha.45 maps it to CFN customMemoryStrategy), so the CLI's four-type
enum was the outlier. A customMemoryStrategy in the --strategies JSON now
converts to { type: 'CUSTOM', name, description, namespaceTemplates }.

The shorthand form still takes managed types only: CUSTOM has no default
namespaces to expand. An extraction configuration or memoryRecordSchema is
rejected rather than dropped, since the CDK schema carries neither.

Also names the offending field in the memory validation error.

* revert: drop CUSTOM memory strategy from "project add memory"

Reverts 87be86e. I added CUSTOM because the CDK schema already had it in
MemoryStrategyTypeSchema, which turns out to be the argument PR #694 made --
and #713 reverted a day later.

The CLI has removed CUSTOM twice on purpose. Offering the type without
somewhere to put its extraction configuration is #241 ("select custom memory
strategy, note there is no option to add prompts"); #266 removed it as a P0 to
stop users picking an unsupported option, #694/#696 added it back with
semanticOverride, and #713 reverted both as premature. #676 tracks doing it
properly. The CDK keeping CUSTOM in its enum without a configuration field is
the same hole, not a licence.

So both forms are rejected again, now with an error that says why and points
at #676. The one thing kept from the reverted commit: memory validation errors
name the offending field, since issue.path was being dropped.

* refactor: drop the long-form help for --description

The one-line flag description is enough; the deploy-time caveat lives in the
PR discussion rather than in help output.

* docs: comment change

* fix: change function name and add comment for clarity

* test: add uncovered unsupported stream content type test

* style: make json example concrete, remove comments

* refactor(project): reuse shared spec validation for memory

* fix(project): validate memory JSON inputs

* fix(project): harden memory input validation

* test(project): colocate memory tests with the add/memory handler

Upstream moved the per-resource `project add` tests out of the monolithic
project.test.ts into colocated add/<resource>/index.test.ts suites (harness
in #2034, online-eval in #2048). Move the memory tests to match, with the
same locally-duplicated run/inProject helpers those suites use.

project.test.ts is now identical to upstream/refactor again, so this PR no
longer touches it. Also drops the DeserializationError, FsReadWriteJson and
ReadWriteJson imports, left dead there once the harness tests that used them
moved to add/harness/index.test.ts.

No test content changed: 187 project tests still pass, now across 10 files
instead of 9.

* refactor(project): match --strategies JSON to the agentcore.json schema

The --strategies flag re-declared its own strategy input schema, modelled
on the CreateMemory API's tagged union (semanticMemoryStrategy et al.) and
requiring a name. agentcore.json stores strategies flat with an optional
name, so the flag accepted a shape the project file never holds and
rejected one it does.

Parse the JSON form with MemoryStrategySchema itself, wrapped only for the
unsupported-field diagnostics, so the flag cannot drift from the schema.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants